Skip to content

Conversation

bastelfreak
Copy link

add basic nginx/unicorn support. nginx vhost is currently based on a template, not on nginx::resource:vhost. tested on centos 7.

@bastelfreak
Copy link
Author

Hey stephenrjohnson, could you take a look at the code?

@stephenrjohnson
Copy link
Owner

I'll get to this on the weekend, sorry about the delay.

@gavinrogers
Copy link

I'm interested in having this merged as well please @stephenrjohnson

@bastelfreak
Copy link
Author

Hey @stephenrjohnson
possible for you to take a look at this?

@arenstar
Copy link

arenstar commented Dec 9, 2014

epel offers python-gunicorn.. ? why compile it?

i think there should be an option to use a local package - i try to avoid compiling on production machines where i can...

@bastelfreak
Copy link
Author

Well, unicorn isn't the same as gunicorn. In general I noticed that the rpms are most of the time outdated so I prefer gems. Also unicorn isn't available via base/epel/extras repos on CentOS7

@arenstar
Copy link

arenstar commented Dec 9, 2014

bastelfreak - i just actually just realised my error there. yep..
Sorry bout that..

Is it possible to have the option of using a package instead though?
ill often build my own packages and distribute them for my systems

@bastelfreak
Copy link
Author

So an option to swtich between the gem or an rpm and the option to specifiy the name of the rpm? Can I assume that the rpm is available via any configured repository? would this suit your needs?

@stephenrjohnson
Copy link
Owner

I'm holding off merging this until you have made the changes, outlined above.

@arenstar
Copy link

arenstar commented Dec 9, 2014

bastelfreak, exactly 👍

in my situation id simply like to offer a rpm package name, which would avoid installing and building using gcc - etc.

Soo setting the package_name

it would also be important to set

installpath - https://github.com/stephenrjohnson/puppetmodule/pull/74/files#diff-85eb9f3c6e80aebeebc8c583f5afd80bR5

and i noticed you arent using the puppet_passenger_port -> https://github.com/stephenrjohnson/puppetmodule/pull/74/files#diff-d52434b9535ec86f061db077733da1fbR23

there are other modules out there which manage repository management.
Lets assume that the rpm should just be available to the system :)

@bastelfreak
Copy link
Author

Can I assume that the binary is always available in your $PATH? Your do you want to specify the complete path to the binary?

I will try to add the changes later this week. are you able to write some unit/rspec tests?

bastelfreak pushed a commit to bastelfreak/puppetmodule that referenced this pull request Jan 22, 2015
@bastelfreak
Copy link
Author

hey @stephenrjohnson could you please take a look at this? implemented the mentioned stuff from @arenstar

let me know if I should rebase it into one commit

@bastelfreak
Copy link
Author

tested this yesterday on several nodes together with @gavinrogers and we fixed two bugs. currently it is running fine on multiple production systems

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know why this has been done but it's a backwards breaking change so we are going to have to make this a major version release.

@stephenrjohnson
Copy link
Owner

The last think we need is some tests as this is a massive change. Have a look in spec/acceptance/ and we don't have any spec tests to test both branches of the webserver tree or even any for the unicorn class.

@bastelfreak
Copy link
Author

anybody with a bit of experience in writing tests around that could assist me? Maybe @gavinrogers ?

@gavinrogers
Copy link

@stephenrjohnson i've started working on spec tests etc. i was curious about the acceptance tests, i noticed that one of the targets is centos7, but i haven't yet found a way to install passenger on centos7, where is the package coming from? i've got it working fine on that platform using the unicorn class, let me get the tests done

@bastelfreak
Copy link
Author

You're writing the tests? Awesome! thanks!

@stephenrjohnson
Copy link
Owner

@gavinrogers If you look at the spec_helper_acceptance

if osfamily == 'RedHat'
on host, puppet('module', 'install', 'stahnma-epel'), { :acceptable_exit_codes => [0,1] }
on host, puppet('apply', '-e', '"class {epel:}"'), { :acceptable_exit_codes => [0] }
#passenger repo
shell("rpm -iv http://yum.theforeman.org/releases/latest/el#{osrelease}/x86_64/foreman-release.rpm")
end

@benfairless
Copy link

Extremely interested in this option. The rest of our stack runs on Nginx & Unicorn/Gunicorn so this would sit within that perfectly.

Obviously the progress on this change has been slow lately. Is there anything I can do to assist in merging it in? Happy to assist with puppet code, testing etc

@bastelfreak
Copy link
Author

please please please write tests. I've no clue about that, would be awesome if you could contribute it.

@benfairless
Copy link

Is that the only change necessary? I'd like to see the vhost for nginx setup using jfryman/nginx. Can do that too if you'd like?

@bastelfreak
Copy link
Author

I can take a look again and try to create a vhost config via jfryman/nginx. Do you know a bit about selinux and can maybe improve the current settings?

@benfairless
Copy link

I've merged all the upstream code and fixed all existing tests (other than ones not passing upstream). Whilst writing unit tests for unicorn though I noticed that none of the parameters for puppet::unicorn have default values at this point. I might have to make some more significant modifications to get it working in a sane manner.

@bastelfreak
Copy link
Author

looks way better now, travis shows only three issues, all not based on this code.

@stephenrjohnson
Copy link
Owner

The issues are caused by your merges not being clean this really needs rebasing on matser.

@bastelfreak
Copy link
Author

rebase is on its way!

@bastelfreak
Copy link
Author

@stephenrjohnson would it be okay for you if I squash this into one commit before rebasing?

@stephenrjohnson
Copy link
Owner

Not the best but sure it's fine.

@bastelfreak
Copy link
Author

Did a huge squash and then rebased against your master

@stephenrjohnson
Copy link
Owner

Why is this still breaking the tests that work before the merge.

@stephenrjohnson stephenrjohnson force-pushed the master branch 2 times, most recently from 7fa12de to 9f61425 Compare May 17, 2015 08:20
this is a huge commit which adds support to install nginx on a centos7 node with unicorn backend. You can run an all-in-one node with puppet master/ca on it, or use it to configure a central ssl offloading proxy with puppet ca on it and several master backends and backups.
@bastelfreak
Copy link
Author

hey @stephenrjohnson, could you have a look again? Did a new rebase and fixed the two travis issues (trailing whitespace and missing variable scope). Can you tell me why travis is failling?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants